release/20.x: [TableGen] Don't use inline storage for ReferenceLocs (NFC) (#125231) #125287

llvmbot · 2025-01-31T20:36:03Z

Backport e2301d6 c640f97

Requested by: @nikic

llvmbot · 2025-01-31T20:36:12Z

@arsenm @jurahul What do you think about merging this PR to the release branch?

llvmbot · 2025-01-31T20:37:24Z

@llvm/pr-subscribers-tablegen

@llvm/pr-subscribers-llvm-globalisel

Author: None (llvmbot)

Changes

Backport e2301d6 c640f97

Requested by: @nikic

Full diff: https://github.com/llvm/llvm-project/pull/125287.diff

3 Files Affected:

(modified) llvm/include/llvm/TableGen/Record.h (+1-1)
(modified) llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp (+17-29)
(modified) llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h (+21-21)

diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index d9930a48e80840..e04ed348231487 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -1523,7 +1523,7 @@ class RecordVal {
   bool IsUsed = false;
 
   /// Reference locations to this record value.
-  SmallVector<SMRange> ReferenceLocs;
+  SmallVector<SMRange, 0> ReferenceLocs;
 
 public:
   RecordVal(const Init *N, const RecTy *T, FieldKind K);
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
index f0cd98dd2dee08..8564bf8d2d91ba 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.cpp
@@ -227,26 +227,12 @@ MatchTableRecord MatchTable::NamedValue(unsigned NumBytes,
                           MatchTableRecord::MTRF_CommaFollows);
 }
 
-MatchTableRecord MatchTable::NamedValue(unsigned NumBytes, StringRef NamedValue,
-                                        int64_t RawValue) {
-  return MatchTableRecord(std::nullopt, NamedValue, NumBytes,
-                          MatchTableRecord::MTRF_CommaFollows, RawValue);
-}
-
 MatchTableRecord MatchTable::NamedValue(unsigned NumBytes, StringRef Namespace,
                                         StringRef NamedValue) {
   return MatchTableRecord(std::nullopt, (Namespace + "::" + NamedValue).str(),
                           NumBytes, MatchTableRecord::MTRF_CommaFollows);
 }
 
-MatchTableRecord MatchTable::NamedValue(unsigned NumBytes, StringRef Namespace,
-                                        StringRef NamedValue,
-                                        int64_t RawValue) {
-  return MatchTableRecord(std::nullopt, (Namespace + "::" + NamedValue).str(),
-                          NumBytes, MatchTableRecord::MTRF_CommaFollows,
-                          RawValue);
-}
-
 MatchTableRecord MatchTable::IntValue(unsigned NumBytes, int64_t IntValue) {
   assert(isUIntN(NumBytes * 8, IntValue) || isIntN(NumBytes * 8, IntValue));
   auto Str = llvm::to_string(IntValue);
@@ -651,8 +637,8 @@ void SwitchMatcher::emit(MatchTable &Table) {
                 [&Table]() { return Table.allocateLabelID(); });
   const unsigned Default = Table.allocateLabelID();
 
-  const int64_t LowerBound = Values.begin()->getRawValue();
-  const int64_t UpperBound = Values.rbegin()->getRawValue() + 1;
+  const int64_t LowerBound = Values.begin()->RawValue;
+  const int64_t UpperBound = Values.rbegin()->RawValue + 1;
 
   emitPredicateSpecificOpcodes(*Condition, Table);
 
@@ -664,10 +650,11 @@ void SwitchMatcher::emit(MatchTable &Table) {
   auto VI = Values.begin();
   for (unsigned I = 0, E = Values.size(); I < E; ++I) {
     auto V = *VI++;
-    while (J++ < V.getRawValue())
+    while (J++ < V.RawValue)
       Table << MatchTable::IntValue(4, 0);
-    V.turnIntoComment();
-    Table << MatchTable::LineBreak << V << MatchTable::JumpTarget(LabelIDs[I]);
+    V.Record.turnIntoComment();
+    Table << MatchTable::LineBreak << V.Record
+          << MatchTable::JumpTarget(LabelIDs[I]);
   }
   Table << MatchTable::LineBreak;
 
@@ -1145,11 +1132,11 @@ void SameOperandMatcher::emitPredicateOpcodes(MatchTable &Table,
 
 std::map<LLTCodeGen, unsigned> LLTOperandMatcher::TypeIDValues;
 
-MatchTableRecord LLTOperandMatcher::getValue() const {
+RecordAndValue LLTOperandMatcher::getValue() const {
   const auto VI = TypeIDValues.find(Ty);
   if (VI == TypeIDValues.end())
     return MatchTable::NamedValue(1, getTy().getCxxEnumValue());
-  return MatchTable::NamedValue(1, getTy().getCxxEnumValue(), VI->second);
+  return {MatchTable::NamedValue(1, getTy().getCxxEnumValue()), VI->second};
 }
 
 bool LLTOperandMatcher::hasValue() const {
@@ -1167,7 +1154,8 @@ void LLTOperandMatcher::emitPredicateOpcodes(MatchTable &Table,
           << MatchTable::ULEB128Value(InsnVarID);
   }
   Table << MatchTable::Comment("Op") << MatchTable::ULEB128Value(OpIdx)
-        << MatchTable::Comment("Type") << getValue() << MatchTable::LineBreak;
+        << MatchTable::Comment("Type") << getValue().Record
+        << MatchTable::LineBreak;
 }
 
 //===- PointerToAnyOperandMatcher -----------------------------------------===//
@@ -1411,12 +1399,12 @@ Error OperandMatcher::addTypeCheckPredicate(const TypeSetByHwMode &VTy,
 DenseMap<const CodeGenInstruction *, unsigned>
     InstructionOpcodeMatcher::OpcodeValues;
 
-MatchTableRecord
+RecordAndValue
 InstructionOpcodeMatcher::getInstValue(const CodeGenInstruction *I) const {
   const auto VI = OpcodeValues.find(I);
   if (VI != OpcodeValues.end())
-    return MatchTable::NamedValue(2, I->Namespace, I->TheDef->getName(),
-                                  VI->second);
+    return {MatchTable::NamedValue(2, I->Namespace, I->TheDef->getName()),
+            VI->second};
   return MatchTable::NamedValue(2, I->Namespace, I->TheDef->getName());
 }
 
@@ -1428,14 +1416,14 @@ void InstructionOpcodeMatcher::initOpcodeValuesMap(
     OpcodeValues[I] = Target.getInstrIntValue(I->TheDef);
 }
 
-MatchTableRecord InstructionOpcodeMatcher::getValue() const {
+RecordAndValue InstructionOpcodeMatcher::getValue() const {
   assert(Insts.size() == 1);
 
   const CodeGenInstruction *I = Insts[0];
   const auto VI = OpcodeValues.find(I);
   if (VI != OpcodeValues.end())
-    return MatchTable::NamedValue(2, I->Namespace, I->TheDef->getName(),
-                                  VI->second);
+    return {MatchTable::NamedValue(2, I->Namespace, I->TheDef->getName()),
+            VI->second};
   return MatchTable::NamedValue(2, I->Namespace, I->TheDef->getName());
 }
 
@@ -1447,7 +1435,7 @@ void InstructionOpcodeMatcher::emitPredicateOpcodes(MatchTable &Table,
         << MatchTable::ULEB128Value(InsnVarID);
 
   for (const CodeGenInstruction *I : Insts)
-    Table << getInstValue(I);
+    Table << getInstValue(I).Record;
   Table << MatchTable::LineBreak;
 }
 
diff --git a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
index e7914a613973b3..77c8bc290faaf3 100644
--- a/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
+++ b/llvm/utils/TableGen/Common/GlobalISel/GlobalISelMatchTable.h
@@ -145,14 +145,10 @@ struct MatchTableRecord {
   /// A bitfield of RecordFlagsBits flags.
   unsigned Flags;
 
-  /// The actual run-time value, if known
-  int64_t RawValue;
-
   MatchTableRecord(std::optional<unsigned> LabelID_, StringRef EmitStr,
-                   unsigned NumElements, unsigned Flags,
-                   int64_t RawValue = std::numeric_limits<int64_t>::min())
+                   unsigned NumElements, unsigned Flags)
       : LabelID(LabelID_.value_or(~0u)), EmitStr(EmitStr),
-        NumElements(NumElements), Flags(Flags), RawValue(RawValue) {
+        NumElements(NumElements), Flags(Flags) {
     assert((!LabelID_ || LabelID != ~0u) &&
            "This value is reserved for non-labels");
   }
@@ -166,12 +162,6 @@ struct MatchTableRecord {
     NumElements = 0;
   }
 
-  /// For Jump Table generation purposes
-  bool operator<(const MatchTableRecord &Other) const {
-    return RawValue < Other.RawValue;
-  }
-  int64_t getRawValue() const { return RawValue; }
-
   void emit(raw_ostream &OS, bool LineBreakNextAfterThis,
             const MatchTable &Table) const;
   unsigned size() const { return NumElements; }
@@ -202,12 +192,8 @@ class MatchTable {
   static MatchTableRecord Comment(StringRef Comment);
   static MatchTableRecord Opcode(StringRef Opcode, int IndentAdjust = 0);
   static MatchTableRecord NamedValue(unsigned NumBytes, StringRef NamedValue);
-  static MatchTableRecord NamedValue(unsigned NumBytes, StringRef NamedValue,
-                                     int64_t RawValue);
   static MatchTableRecord NamedValue(unsigned NumBytes, StringRef Namespace,
                                      StringRef NamedValue);
-  static MatchTableRecord NamedValue(unsigned NumBytes, StringRef Namespace,
-                                     StringRef NamedValue, int64_t RawValue);
   static MatchTableRecord IntValue(unsigned NumBytes, int64_t IntValue);
   static MatchTableRecord ULEB128Value(uint64_t IntValue);
   static MatchTableRecord Label(unsigned LabelID);
@@ -400,6 +386,20 @@ class GroupMatcher final : public Matcher {
   bool candidateConditionMatches(const PredicateMatcher &Predicate) const;
 };
 
+/// MatchTableRecord and associated value, for jump table generation.
+struct RecordAndValue {
+  MatchTableRecord Record;
+  int64_t RawValue;
+
+  RecordAndValue(MatchTableRecord Record,
+                 int64_t RawValue = std::numeric_limits<int64_t>::min())
+      : Record(std::move(Record)), RawValue(RawValue) {}
+
+  bool operator<(const RecordAndValue &Other) const {
+    return RawValue < Other.RawValue;
+  }
+};
+
 class SwitchMatcher : public Matcher {
   /// All the nested matchers, representing distinct switch-cases. The first
   /// conditions (as Matcher::getFirstCondition() reports) of all the nested
@@ -414,7 +414,7 @@ class SwitchMatcher : public Matcher {
 
   /// Temporary set used to check that the case values don't repeat within the
   /// same switch.
-  std::set<MatchTableRecord> Values;
+  std::set<RecordAndValue> Values;
 
   /// An owning collection for any auxiliary matchers created while optimizing
   /// nested matchers contained.
@@ -874,7 +874,7 @@ class PredicateMatcher {
     return hasValue() && PredicateMatcher::isIdentical(B);
   }
 
-  virtual MatchTableRecord getValue() const {
+  virtual RecordAndValue getValue() const {
     assert(hasValue() && "Can not get a value of a value-less predicate!");
     llvm_unreachable("Not implemented yet");
   }
@@ -968,7 +968,7 @@ class LLTOperandMatcher : public OperandPredicateMatcher {
            Ty == cast<LLTOperandMatcher>(&B)->Ty;
   }
 
-  MatchTableRecord getValue() const override;
+  RecordAndValue getValue() const override;
   bool hasValue() const override;
 
   LLTCodeGen getTy() const { return Ty; }
@@ -1378,7 +1378,7 @@ class InstructionOpcodeMatcher : public InstructionPredicateMatcher {
 
   static DenseMap<const CodeGenInstruction *, unsigned> OpcodeValues;
 
-  MatchTableRecord getInstValue(const CodeGenInstruction *I) const;
+  RecordAndValue getInstValue(const CodeGenInstruction *I) const;
 
 public:
   static void initOpcodeValuesMap(const CodeGenTarget &Target);
@@ -1405,7 +1405,7 @@ class InstructionOpcodeMatcher : public InstructionPredicateMatcher {
 
   // TODO: This is used for the SwitchMatcher optimization. We should be able to
   // return a list of the opcodes to match.
-  MatchTableRecord getValue() const override;
+  RecordAndValue getValue() const override;
 
   void emitPredicateOpcodes(MatchTable &Table,
                             RuleMatcher &Rule) const override;

nikic · 2025-01-31T20:43:33Z

This backport is to fix TableGen OOM seen on i686-mingw.

Update to LLVM 20 LLVM 20 GA is scheduled for March 11th. Rust 1.86 will be stable on April 3rd. * [x] rust-lang#135764 * [x] rust-lang#136134 * [x] rust-lang/compiler-builtins#752 * [ ] llvm/llvm-project#125287 * [ ] Update compiler-builtins Tested: host-x86_64, host-aarch64, mingw try-job: x86_64-msvc-1 try-job: x86_64-msvc-2 try-job: i686-msvc-1 try-job: i686-msvc-2 try-job: dist-x86_64-msvc try-job: dist-i686-msvc try-job: dist-aarch64-msvc

jurahul

LGTM but also let @arsenm take a look for the GISel match tables changes (I did not review that PR).

Update to LLVM 20 LLVM 20 GA is scheduled for March 11th. Rust 1.86 will be stable on April 3rd. * [x] rust-lang#135764 * [x] rust-lang#136134 * [x] rust-lang/compiler-builtins#752 * [ ] llvm/llvm-project#125287 * [ ] Update compiler-builtins (blocked on rust-lang#135501) Tested: host-x86_64, host-aarch64, apple, mingw try-job: x86_64-msvc-1 try-job: i686-msvc-1 try-job: dist-aarch64-msvc try-job: x86_64-msvc-ext1 try-job: x86_64-msvc-ext2 try-job: x86_64-msvc-ext3

nikic · 2025-02-07T08:27:39Z

ping @arsenm

MatchTableRecord stores a 64-bit RawValue. However, this field is only needed by a small part of the code (jump table generation). Create a separate RecordAndValue structure that is used in just the necessary places. Based on massif, this reduces memory usage on RISCVGenGlobalISel.inc by about 100MB (to 2.15GB). (cherry picked from commit e2301d6)

) The ReferenceLocs are not enabled by default (they are used by the tablegen lsp server), and as such always empty, but still allocate inline storage for the SmallVector. Disabling it saves about 200MB on RISCVGenGlobalISel.inc. (The equivalent field in Record already disables inline storage.) (cherry picked from commit c640f97)

github-actions · 2025-02-07T20:04:23Z

@nikic (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Update to LLVM 20 LLVM 20 GA is scheduled for March 11th. Rust 1.87 will be stable on May 15th. * [x] rust-lang#135764 * [x] rust-lang#136134 * [x] rust-lang/compiler-builtins#752 * [x] llvm/llvm-project#125287 * [x] rust-lang#136537 * [x] rust-lang#136895 * [x] Wait for beta branch (Feb 14). Tested: host-x86_64, host-aarch64, apple, mingw, msvc

Update to LLVM 20 LLVM 20 GA is scheduled for March 11th. Rust 1.87 will be stable on May 15th. * [x] rust-lang/rust#135764 * [x] rust-lang/rust#136134 * [x] rust-lang/compiler-builtins#752 * [x] llvm/llvm-project#125287 * [x] rust-lang/rust#136537 * [x] rust-lang/rust#136895 * [x] Wait for beta branch (Feb 14). Tested: host-x86_64, host-aarch64, apple, mingw, msvc

llvmbot added this to the LLVM 20.X Release milestone Jan 31, 2025

llvmbot requested review from arsenm and jurahul January 31, 2025 20:36

llvmbot added tablegen llvm:globalisel labels Jan 31, 2025

llvmbot mentioned this pull request Jan 31, 2025

[TableGen] Reduce size of MatchTableRecord (NFC) #125221

Merged

nikic mentioned this pull request Jan 31, 2025

Update to LLVM 20 rust-lang/rust#135763

Merged

7 tasks

jurahul approved these changes Jan 31, 2025

View reviewed changes

arsenm approved these changes Feb 7, 2025

View reviewed changes

nikic added 2 commits February 7, 2025 12:03

llvmbot force-pushed the issue125221 branch from 96f6234 to 4d04a40 Compare February 7, 2025 20:03

tstellar merged commit 4d04a40 into llvm:release/20.x Feb 7, 2025
7 of 10 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

release/20.x: [TableGen] Don't use inline storage for ReferenceLocs (NFC) (#125231) #125287

release/20.x: [TableGen] Don't use inline storage for ReferenceLocs (NFC) (#125231) #125287

Uh oh!

llvmbot commented Jan 31, 2025

Uh oh!

llvmbot commented Jan 31, 2025

Uh oh!

llvmbot commented Jan 31, 2025 •

edited

Loading

Uh oh!

nikic commented Jan 31, 2025

Uh oh!

jurahul left a comment

Uh oh!

nikic commented Feb 7, 2025

Uh oh!

Uh oh!

github-actions bot commented Feb 7, 2025

Uh oh!

Uh oh!

release/20.x: [TableGen] Don't use inline storage for ReferenceLocs (NFC) (#125231) #125287

release/20.x: [TableGen] Don't use inline storage for ReferenceLocs (NFC) (#125231) #125287

Uh oh!

Conversation

llvmbot commented Jan 31, 2025

Uh oh!

llvmbot commented Jan 31, 2025

Uh oh!

llvmbot commented Jan 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic commented Jan 31, 2025

Uh oh!

jurahul left a comment

Choose a reason for hiding this comment

Uh oh!

nikic commented Feb 7, 2025

Uh oh!

Uh oh!

github-actions bot commented Feb 7, 2025

Uh oh!

Uh oh!

llvmbot commented Jan 31, 2025 •

edited

Loading